Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/es module support #38

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

elpoelma
Copy link
Collaborator

Adds support for the installation of es modules in your project.


"compilerOptions": {
"lib": ["es2020"],
"module": "commonjs",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes indeed

tsconfig-base.json Outdated Show resolved Hide resolved
@nvdk
Copy link
Member

nvdk commented Mar 31, 2022

you may want to change the base to feature/typescript-support to make the diff a bit cleaner. Github will automatically update the base when the other PR is merged

@elpoelma elpoelma changed the base branch from master to feature/typescript-support March 31, 2022 14:21
@madnificent
Copy link
Member

Indentation changes make it harder to read this PR, regardless of whether or not they should be executed at all.

Is the change in the import statements something that end-users will notice too? We see the import changed from ./server and ./sparql to ./server.js and ./sparql.js or is that an automatic change? Is there a way to keep this consistent with the way the template worked and lower the noise? :-)

@erikap erikap changed the base branch from feature/typescript-support to master April 6, 2022 11:53
@elpoelma
Copy link
Collaborator Author

elpoelma commented Apr 7, 2022

The .js extensions which are added are IIRC a requirement when using es modules.

@nvdk
Copy link
Member

nvdk commented May 23, 2022

The .js extensions which are added are IIRC a requirement when using es modules.

as per discussion in the chat, this is confirmed by https://nodejs.org/api/esm.html#mandatory-file-extensions . As such we should consider this a breaking feature

@nvdk
Copy link
Member

nvdk commented May 23, 2022

as per further discussion with @MikiDi, I think we could restore/support the old behaviour with babel using this plugin: https://www.npmjs.com/package/babel-plugin-add-import-extension

@Windvis
Copy link

Windvis commented Aug 24, 2023

If a breaking change is happening with the node 18 bump anyways, would it make sense to include this as well? More and more packages are switching to ESM-only publishing setups.

@MikiDi
Copy link
Contributor

MikiDi commented Aug 25, 2023

This still would need some work then, right (swapping target to es2022, ...) ? The changes themselves should be easy, but I don't feel confident as a tester for this one.

@Windvis Windvis mentioned this pull request Mar 25, 2024
@Windvis
Copy link

Windvis commented Mar 25, 2024

(I've opened #63 to discuss the implications and the plan forward.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants